-
Notifications
You must be signed in to change notification settings - Fork 1
[Do not merge!] Pseudo PR for first release #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: TEMPLATE
Are you sure you want to change the base?
Conversation
Important! Template update for nf-core/tools v3.4.1
One way I would suggest is to include config files under the includeConfig {
if (params.method == "method1") {
"conf/method1.config"
} else if (params.method == "method2") {
"conf/method2.config"
} else {
"/dev/null"
}
}.call()This is similar to using config profiles The only difference is that profiles can be combined and can overwrite settings of another depending on order. |
Update to new runner size syntax
|
Hello @mashehu, How do you think we should proceed? @mahesh-panchal also proposed an alternative (see comment above), what do you think of this? It's very similar to the current behavior with |
|
From what I've seen, this looks like a perfect situation for profiles as that's exactly what they are meant to be used for. I'm also not sure dynamic configuration includes will still be allowed in later versions of Nextflow so profiles seem to be the most reliable way to fix this issue I think |
|
Thanks @nvnieuwk for your comment! So, you prefer profiles over NB: it won't solve the schema display on the nf-core website though, since we'll still have nested params |
|
Yes, these nested parameters need to be flattened i am afraid. Or converted to maps. |
@nvnieuwk Is this feeling or based on a conversation somewhere? Removing dynamic configuration would complicate a lot of things, including the use of institutional configs. |
These parameters are already maps, as far as I understand, no? Again, I'm new to Groovy, so maybe I didn't catch something, but it seems to work well. E.g., I can overwrite a specific nested param via the command line like Is that enough to use maps, @mashehu? Flattening the nested params would result in a lot of unnecessary complexity and maintenance challenges (I can provide more details why, if you want). |
This is a feeling, I'm just a bit wary with these "unsupported" ways of using configuration as these might stop working accidently. Might be something worth discussing with Ben though |
Preferably yes, this way the default configurations are shipped with the pipeline, while params files usually are not shipped with the pipeline. You can still write a small script to convert your snakemake config file to a nextflow config file. That shouldn't be too difficult to do since the only things changed are the parameters. |
Yes, it's pretty easy to do, indeed. It just means that when adding a new feature, I'll always need to ensure that both config files (Snakemake and Nextflow) are updated in the same way. Not very complicated though, I agree. |
|
Great, I'd be happy to review this change if you want :) |
|
Hi @nvnieuwk, I made a small script to generate all the profiles, you can see the result in this commit If it looks good to you, I can add a description for all these profiles in |
|
Looks good! It might be a good idea to also add the script to the pipeline so other people can also update this later on |
|
Alright, I added these scripts and also added a small comment for every profile in the |
Do not merge! This is a PR of dev compared to the TEMPLATE branch for whole-pipeline reviewing purposes. Changes should be made to dev and this PR should not be merged! The actual release PR is at